cli parity #147 with fixes - #216

CLI and output parity from @BenjaminDEMAILLE — 31 STAR 2.7.11b parameters, and a test that turns the parameter surface into a machine-checked number. His commits are preserved with original authorship; my fixes follow as their own commits.

Closes #147

What's in it

tests/parameter_surface.rs checks every parameter name STAR 2.7.11b accepts against what clap recognises — all 203 column-1 names from parametersDefault, interface only. Two supporting lists stop "accepted" meaning "implemented": ACCEPTED_BUT_INERT (knobs that parse but change no output byte, each with a reason) and NOT_YET_ACCEPTED (the real gap, grouped by theme). The test fails in both directions — an unrecognised name that is not declared missing, and a name declared missing that has since been implemented.

Flags that now do something: --outSAMmode None/NoQS, --outSJtype None, --outSJfilterReads Unique, --outSAMheaderHD/PG/CommentFile, --readFilesPrefix, --readQualityScoreBase, --outQSconversionAdd, --readNameSeparator.

Changes made while merging

--outSJfilterReads Unique was filtering in the wrong place. It discounted multi-mapping reads at output time, zeroing column 8 while leaving column 9 (max spliced overhang) built from every read. Against STAR on 10k yeast reads, junction II:332876 came out 1 0 57 where STAR gives 1 0 14 — same counts, an overhang no unique read supports.

STAR gates the recording instead: ReadAlign::recordSJ skips the read entirely unless outSJfilterReads=="All" || nTrO==1, so a multimapper contributes nothing — not its counts, not its overhang. SpliceJunctionStats now carries the flag and record_junction returns early for a non-unique read, which makes the output-time discount redundant.

Result: under the flag, SJ.out.tab differs from STAR on 3 lines instead of 5, and those 3 are exactly the ones that already differ at defaults — so the flag no longer adds divergence of its own.

Two implemented parameters were listed as inert. runRNGseed feeds per_read_seed, which seeds the shuffle under --outMultimapperOrder Random: the same 10k reads at --runRNGseed 777 versus 424242 give 1444 differing records. Its stated reason ("seeded per read, not from a global RNG") described the mechanism and drew the wrong conclusion — the per-read seed is derived from the global one. limitBAMsortRAM is enforced in io/bam.rs and ends the run: limitBAMsortRAM=1000 bytes exceeded: estimated 3889200 bytes for 9723 records.

Both are still accepted, so they simply leave the list. I checked the rest of it too: limitGenomeGenerateRAM looks used but only logs "accepted but not enforced", so it is genuinely inert and stays.

The list's inert half is not machine-checked — inert_parameters_are_actually_accepted verifies each name is accepted, never that it is unread. That gap is what let these two sit there, and it is now noted in the doc comment so the next entry is added deliberately.

Three stale NOT_YET_ACCEPTED entries. genomeChrSetMitochondrial, soloCellReadStats and soloClusterCBfile landed in #200 and #212. The surface test caught them itself on its first run against current main, which is the drift it exists to catch.

The default output change had no test that could fail. See below.

Corrections to the PR description

Two figures in #147's description no longer hold, both because the tree moved under it:

• It says ACCEPTED_BUT_INERT holds 22 knobs. It held 16; after removing the two implemented ones it holds 14.
• It quotes 175/203 accepted. It is 178/203, because Batches B and C landed three parameters that were still listed as missing.

Both are now printed by report_coverage rather than counted by hand:

STAR 2.7.11b parameter surface: 178/203 accepted (14 inert), 25 not yet accepted

Worth noting the description also opens with a self-correction, retracting an earlier revision that had trimmed three names out of the fixture and quietly shrunk the denominator.

The one output change at default settings

STAR's default --readNameSeparator is / and STAR cuts there; this codebase did not.

Verified against STAR on reads whose QNAMEs carry a /1: every QNAME matches STAR's.

test_read_name_separator_cuts_the_qname_and_is_configurable is an added test for --readNameSeparator -. Stops a hardcoded / passing as flag-driven behaviour. Confirmed it fails when the separator list is cleared.

Validation

Defaults byte-identical to main. Records compared with headers excluded, since the @PG CL: line necessarily differs by output prefix:

Against STAR 2.7.11b, unchanged from the recorded baselines:

Reproduce the differentials with the dataset from CONTRIBUTING:

./target/release/rustar-aligner --runMode alignReads --genomeDir "$DATA/indices_rustar" \
  --readFilesIn "$DATA/reads/ERR12389696_sub_1_10k.fastq.gz" --readFilesCommand zcat \
  --outFileNamePrefix out_se_ --outSAMtype SAM --runThreadN 1
python3 test/compare_sam.py --rustar-aligner out_se_Aligned.out.sam \
  --star "$DATA/star_10k_/Aligned.out.sam"

Note for whichever merges second

Four flags in NOT_YET_ACCEPTED here (--genomeType, --genomeTransformOutput, --genomeSuffixLengthMax, --sjdbInsertSave) are implemented on #109 and #160. The surface test will fail on whichever lands second until they are moved out of that list — by design.